feat(diagnostic): превышение длины ключа индекса для файловой ИБ (#3986) - #4292
Open
erprivalov wants to merge 1 commit into
Open
feat(diagnostic): превышение длины ключа индекса для файловой ИБ (#3986)#4292erprivalov wants to merge 1 commit into
erprivalov wants to merge 1 commit into
Conversation
Contributor
📝 WalkthroughWalkthroughAdds the ChangesFile DB index key diagnostics
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Metadata
participant Diagnostic
participant SizeCalculation
participant Report
Metadata->>Diagnostic: check register metadata
Diagnostic->>SizeCalculation: calculate index key byte sizes
SizeCalculation-->>Diagnostic: return composite and individual index sizes
Diagnostic->>Report: emit FILE or MSSQL violation
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
docs/diagnostics/FileDbIndexKeyLengthExceeded.md (1)
32-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the complete configuration contract in both languages.
The new
maxIndexKeyLengthBytesoverride and the MSSQL 16-field limit are missing from both diagnostic pages.
docs/diagnostics/FileDbIndexKeyLengthExceeded.md#L32-L37: document the file-limit override and the MSSQL field-count restriction in Russian.docs/en/diagnostics/FileDbIndexKeyLengthExceeded.md#L28-L33: document the same behavior in English.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/diagnostics/FileDbIndexKeyLengthExceeded.md` around lines 32 - 37, Update the configuration sections in docs/diagnostics/FileDbIndexKeyLengthExceeded.md (lines 32-37) and docs/en/diagnostics/FileDbIndexKeyLengthExceeded.md (lines 28-33) to document maxIndexKeyLengthBytes as the file-limit override and state the MSSQL restriction of no more than 16 fields, providing equivalent complete guidance in Russian and English.Source: Coding guidelines
src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/FileDbIndexKeyLengthExceededDiagnosticTest.java (1)
45-64: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExpand coverage beyond diagnostic counts.
These tests only assert list sizes for one fixture. Add assertions for calculated size, index name, fields, limit, and message content, plus cases for exact/just-over-limit boundaries,
maxIndexKeyLengthBytes, periodic registers, composite types, non-periodic registers, and the MSSQL 16-field limit. As per coding guidelines, diagnostic tests must cover edge cases comprehensively.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/FileDbIndexKeyLengthExceededDiagnosticTest.java` around lines 45 - 64, Expand the test methods in FileDbIndexKeyLengthExceededDiagnosticTest around testFileMode, testMssqlMode, and testAllMode to validate each diagnostic’s calculated size, index name, fields, limit, and message content rather than only counts. Add focused cases covering exact and just-over-limit boundaries, maxIndexKeyLengthBytes, periodic and non-periodic registers, composite types, and the MSSQL 16-field limit, preserving assertions for the expected diagnostic count and values in each mode.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/diagnostics/FileDbIndexKeyLengthExceeded.md`:
- Around line 1-6: Remove the duplicate H1 diagnostic heading in
FileDbIndexKeyLengthExceeded.md by retaining the generated heading and deleting
the repeated heading under “Описание диагностики,” ensuring the document has
only one top-level title and passes MD024.
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/FileDbIndexKeyLengthExceededDiagnostic.java`:
- Around line 130-140: The checkAndReport method must also enforce the MSSQL
16-field limit, not only the byte-size limit. Pass the relevant field count from
the ByDims validation flow into checkAndReport, and add an MSSQL diagnostic when
the count exceeds 16 while preserving the existing byte-limit and FILE checks.
- Around line 82-85: Update the type guard in checkMetadata so analysis
continues only for InformationRegister and CalculationRegister instances, while
returning immediately for all other Register kinds. Preserve the existing
register processing for the two supported types.
- Around line 44-64: The public API in FileDbIndexKeyLengthExceededDiagnostic
needs contract-level Javadoc. Document the class’s supported metadata types,
index-key limits, configuration parameters maxIndexKeyLengthBytes and checkMode,
valid CheckMode values, defaults, and any diagnostic side effects; add Javadoc
to the public class, enum, and exposed configuration members without changing
behavior.
- Around line 135-140: Update checkAndReport in
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/FileDbIndexKeyLengthExceededDiagnostic.java#L135-L140
to pass currentSize to getMessage after the configured limit, preserving
argument order. Add the matching actual-size placeholder to both localized
messages in
src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/FileDbIndexKeyLengthExceededDiagnostic_en.properties#L2-L2
and
src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/FileDbIndexKeyLengthExceededDiagnostic_ru.properties#L2-L2.
In
`@src/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/parameters-schema.json`:
- Around line 765-787: Constrain the FileDbIndexKeyLengthExceeded properties:
add an enum permitting only FILE, MSSQL, and ALL for checkMode, and require
maxIndexKeyLengthBytes to be a non-negative integer by setting its minimum to 0.
Preserve the existing defaults and descriptions.
---
Nitpick comments:
In `@docs/diagnostics/FileDbIndexKeyLengthExceeded.md`:
- Around line 32-37: Update the configuration sections in
docs/diagnostics/FileDbIndexKeyLengthExceeded.md (lines 32-37) and
docs/en/diagnostics/FileDbIndexKeyLengthExceeded.md (lines 28-33) to document
maxIndexKeyLengthBytes as the file-limit override and state the MSSQL
restriction of no more than 16 fields, providing equivalent complete guidance in
Russian and English.
In
`@src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/FileDbIndexKeyLengthExceededDiagnosticTest.java`:
- Around line 45-64: Expand the test methods in
FileDbIndexKeyLengthExceededDiagnosticTest around testFileMode, testMssqlMode,
and testAllMode to validate each diagnostic’s calculated size, index name,
fields, limit, and message content rather than only counts. Add focused cases
covering exact and just-over-limit boundaries, maxIndexKeyLengthBytes, periodic
and non-periodic registers, composite types, and the MSSQL 16-field limit,
preserving assertions for the expected diagnostic count and values in each mode.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fa0c273f-034d-4fb6-bc20-0bb3aba65f01
⛔ Files ignored due to path filters (5)
src/test/resources/diagnostics/FileDbIndexKeyLengthExceededDiagnostic.bslis excluded by!src/test/resources/**src/test/resources/metadata/FileDbIndexKeyLengthExceeded/Configuration.xmlis excluded by!src/test/resources/**src/test/resources/metadata/FileDbIndexKeyLengthExceeded/InformationRegisters/ВалидныйРегистрСКороткимиИзмерениями.xmlis excluded by!src/test/resources/**src/test/resources/metadata/FileDbIndexKeyLengthExceeded/InformationRegisters/НевалидныйРегистрСДлиннойСтрокой.xmlis excluded by!src/test/resources/**src/test/resources/metadata/FileDbIndexKeyLengthExceeded/InformationRegisters/РегистрНарушающийТолькоЛимитMSSQL.xmlis excluded by!src/test/resources/**
📒 Files selected for processing (8)
docs/diagnostics/FileDbIndexKeyLengthExceeded.mddocs/en/diagnostics/FileDbIndexKeyLengthExceeded.mdsrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/FileDbIndexKeyLengthExceededDiagnostic.javasrc/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/parameters-schema.jsonsrc/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/schema.jsonsrc/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/FileDbIndexKeyLengthExceededDiagnostic_en.propertiessrc/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/FileDbIndexKeyLengthExceededDiagnostic_ru.propertiessrc/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/FileDbIndexKeyLengthExceededDiagnosticTest.java
Comment on lines
+1
to
+6
| # Превышена максимальная длина ключа индекса (FileDbIndexKeyLengthExceeded) | ||
|
|
||
| <!-- Блоки выше заполняются автоматически, не трогать --> | ||
| ## Описание диагностики | ||
|
|
||
| # Превышена максимальная длина ключа индекса (FileDbIndexKeyLengthExceeded) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the duplicate diagnostic heading.
The same H1 appears at Line [1] and Line [6], and markdownlint reports MD024. Keep only the generated heading or rename the second heading so the documentation passes lint.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 6-6: Multiple headings with the same content
(MD024, no-duplicate-heading)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/diagnostics/FileDbIndexKeyLengthExceeded.md` around lines 1 - 6, Remove
the duplicate H1 diagnostic heading in FileDbIndexKeyLengthExceeded.md by
retaining the generated heading and deleting the repeated heading under
“Описание диагностики,” ensuring the document has only one top-level title and
passes MD024.
Source: Linters/SAST tools
Comment on lines
+44
to
+64
| @DiagnosticMetadata( | ||
| type = DiagnosticType.ERROR, | ||
| severity = DiagnosticSeverity.CRITICAL, | ||
| minutesToFix = 30, | ||
| tags = {DiagnosticTag.STANDARD, DiagnosticTag.DESIGN, DiagnosticTag.ERROR} | ||
| ) | ||
| @Getter | ||
| @Setter | ||
| public class FileDbIndexKeyLengthExceededDiagnostic extends AbstractMetadataDiagnostic { | ||
|
|
||
| private static final int MSSQL_LIMIT = 900; | ||
|
|
||
| @DiagnosticParameter(type = Integer.class, defaultValue = "1920") | ||
| private int maxIndexKeyLengthBytes = 1920; | ||
|
|
||
| public enum CheckMode { | ||
| FILE, MSSQL, ALL | ||
| } | ||
|
|
||
| @DiagnosticParameter(type = String.class, defaultValue = "ALL") | ||
| private CheckMode checkMode = CheckMode.ALL; |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add contract-level Javadoc for the public diagnostic API.
The public diagnostic class and its public configuration enum/parameters lack documentation describing supported metadata types, limits, configuration values, and side effects. As per coding guidelines, public Java APIs must document their contracts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/FileDbIndexKeyLengthExceededDiagnostic.java`
around lines 44 - 64, The public API in FileDbIndexKeyLengthExceededDiagnostic
needs contract-level Javadoc. Document the class’s supported metadata types,
index-key limits, configuration parameters maxIndexKeyLengthBytes and checkMode,
valid CheckMode values, defaults, and any diagnostic side effects; add Javadoc
to the public class, enum, and exposed configuration members without changing
behavior.
Source: Coding guidelines
Comment on lines
+82
to
+85
| public void checkMetadata(MD md) { | ||
| if (!(md instanceof Register register)) { | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restrict analysis to information and calculation registers.
The guard accepts every Register; only periodicity handling is narrowed to InformationRegister and CalculationRegister. As a result, other register kinds covered by the common base can also be reported, contrary to the diagnostic contract and documentation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/FileDbIndexKeyLengthExceededDiagnostic.java`
around lines 82 - 85, Update the type guard in checkMetadata so analysis
continues only for InformationRegister and CalculationRegister instances, while
returning immediately for all other Register kinds. Preserve the existing
register processing for the two supported types.
Comment on lines
+130
to
+140
| // Шаг 4: Проверка основного составного индекса (ByDims) | ||
| String byDimsDetails = String.join(", ", byDimsFields); | ||
| checkAndReport("ByDims", byDimsDetails, mainIndexSize); | ||
| } | ||
|
|
||
| private void checkAndReport(String indexName, String fieldsDetail, int currentSize) { | ||
| if ((checkMode == CheckMode.MSSQL || checkMode == CheckMode.ALL) && currentSize > MSSQL_LIMIT) { | ||
| addDiagnostic(getInfo().getMessage(indexName, fieldsDetail, "MSSQL", MSSQL_LIMIT)); | ||
| } | ||
| if ((checkMode == CheckMode.FILE || checkMode == CheckMode.ALL) && currentSize > this.maxIndexKeyLengthBytes) { | ||
| addDiagnostic(getInfo().getMessage(indexName, fieldsDetail, "FILE", this.maxIndexKeyLengthBytes)); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Enforce the MSSQL 16-field index limit.
checkAndReport validates only the byte limit and receives no field count, so a ByDims index with 17 or more fields can pass when its byte size is at most 900. The PR objective explicitly requires the MSSQL field-count restriction; pass the field count and report this violation as well.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/FileDbIndexKeyLengthExceededDiagnostic.java`
around lines 130 - 140, The checkAndReport method must also enforce the MSSQL
16-field limit, not only the byte-size limit. Pass the relevant field count from
the ByDims validation flow into checkAndReport, and add an MSSQL diagnostic when
the count exceeds 16 while preserving the existing byte-limit and FILE checks.
Comment on lines
+135
to
+140
| private void checkAndReport(String indexName, String fieldsDetail, int currentSize) { | ||
| if ((checkMode == CheckMode.MSSQL || checkMode == CheckMode.ALL) && currentSize > MSSQL_LIMIT) { | ||
| addDiagnostic(getInfo().getMessage(indexName, fieldsDetail, "MSSQL", MSSQL_LIMIT)); | ||
| } | ||
| if ((checkMode == CheckMode.FILE || checkMode == CheckMode.ALL) && currentSize > this.maxIndexKeyLengthBytes) { | ||
| addDiagnostic(getInfo().getMessage(indexName, fieldsDetail, "FILE", this.maxIndexKeyLengthBytes)); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Report the calculated key size in every localized diagnostic message.
The Java call currently does not pass currentSize, while both bundles only render the configured limit. Keep the placeholder and argument order identical across all three sites.
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/FileDbIndexKeyLengthExceededDiagnostic.java#L135-L140: pass the calculated size togetMessage.src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/FileDbIndexKeyLengthExceededDiagnostic_en.properties#L2-L2: add a placeholder for the actual size.src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/FileDbIndexKeyLengthExceededDiagnostic_ru.properties#L2-L2: add the same actual-size placeholder.
📍 Affects 3 files
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/FileDbIndexKeyLengthExceededDiagnostic.java#L135-L140(this comment)src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/FileDbIndexKeyLengthExceededDiagnostic_en.properties#L2-L2src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/FileDbIndexKeyLengthExceededDiagnostic_ru.properties#L2-L2
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/FileDbIndexKeyLengthExceededDiagnostic.java`
around lines 135 - 140, Update checkAndReport in
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/FileDbIndexKeyLengthExceededDiagnostic.java#L135-L140
to pass currentSize to getMessage after the configured limit, preserving
argument order. Add the matching actual-size placeholder to both localized
messages in
src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/FileDbIndexKeyLengthExceededDiagnostic_en.properties#L2-L2
and
src/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/FileDbIndexKeyLengthExceededDiagnostic_ru.properties#L2-L2.
Comment on lines
+765
to
+787
| "FileDbIndexKeyLengthExceeded": { | ||
| "description": "Index key length limit exceeded", | ||
| "default": true, | ||
| "type": [ | ||
| "boolean", | ||
| "object" | ||
| ], | ||
| "title": "Index key length limit exceeded", | ||
| "properties": { | ||
| "checkMode": { | ||
| "description": "Limit check mode (FILE, MSSQL, ALL)", | ||
| "default": "ALL", | ||
| "type": "string", | ||
| "title": "Limit check mode (FILE, MSSQL, ALL)" | ||
| }, | ||
| "maxIndexKeyLengthBytes": { | ||
| "description": "Maximum index key length (in bytes) for File DB", | ||
| "default": 1920, | ||
| "type": "integer", | ||
| "title": "Maximum index key length (in bytes) for File DB" | ||
| } | ||
| }, | ||
| "$id": "#/definitions/FileDbIndexKeyLengthExceeded" |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Constrain the new configuration values.
checkMode is only typed as an arbitrary string, so typos and lowercase values pass schema validation and are silently converted to ALL by configure. Also, maxIndexKeyLengthBytes accepts negative integers, which makes every index appear to exceed the configured limit. Add an enum for FILE/MSSQL/ALL and a non-negative minimum for the byte limit.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/parameters-schema.json`
around lines 765 - 787, Constrain the FileDbIndexKeyLengthExceeded properties:
add an enum permitting only FILE, MSSQL, and ALL for checkMode, and require
maxIndexKeyLengthBytes to be a non-negative integer by setting its minimum to 0.
Preserve the existing defaults and descriptions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Описание
Добавлена новая метаданная диагностика
FileDbIndexKeyLengthExceeded, которая проверяет физический размер ключей индексов для регистров сведений и регистров расчета.Платформа 1С может падать с ошибкой СУБД при обновлении конфигурации или переходе в файловый вариант, если суммарный размер ключа индекса превышает лимиты (1920 байт для файловой БД, 900 байт для MS SQL). Данная диагностика позволяет обнаружить эту проблему на этапе написания или проверки кода.
Что реализовано:
_TRef/_RRefи маркеры составных типов).Период(8 байт) для периодических регистров.ByDims), состоящего из всех его измерений.checkMode(FILE,MSSQL,ALL) для выбора целевой СУБД и возможность переопределить лимит для файловой базы черезmaxIndexKeyLengthBytes.Связанные задачи
Closes #3986
Чеклист
Общие
gradlew precommit)Для диагностик
Дополнительно
Summary by CodeRabbit
New Features
Documentation
Configuration